Just as a side note: a user who wants to build individual packages without ament_tools will need to specify CMake arguments in order to build share libraries.

Have you tried to use static libraries when more than one rmw implementation is present?

The reworked typesupport system relies on building shared libraries which are then dlopened at runtime (see ros2/rosidl#182). I would assume using static libraries won't work until a specialization is being implemented which bypasses that mechanism which will only be possible if only a single rmw impl. is present.

@dirk-thomas I was under the assumption that building without ament wasn't encouraged, if someone is brave enough to build a package without it, there's a chance they know how to pass BUILD_SHARED_LIBS to CMake ;-)

Yes, I've built ROS2 statically with FastRTPS and OpenSplice, and typesupport libraries along with messages are built for both rmw implementations:

$ find install_isolated/ -name "*.a"
install_isolated//builtin_interfaces/lib/libbuiltin_interfaces__rosidl_generator_c.a
install_isolated//builtin_interfaces/lib/libbuiltin_interfaces__rosidl_typesupport_cpp.a
install_isolated//builtin_interfaces/lib/libbuiltin_interfaces__rosidl_typesupport_introspection_c.a
install_isolated//builtin_interfaces/lib/libbuiltin_interfaces__rosidl_typesupport_introspection_cpp.a
install_isolated//builtin_interfaces/lib/libbuiltin_interfaces__rosidl_typesupport_opensplice_c.a
install_isolated//builtin_interfaces/lib/libbuiltin_interfaces__rosidl_typesupport_opensplice_cpp.a
install_isolated//fastcdr/lib/libfastcdr.a
install_isolated//fastrtps/lib/libfastrtps.a
install_isolated//rmw/lib/librmw.a
install_isolated//rmw_fastrtps_cpp/lib/librmw_fastrtps_cpp.a
install_isolated//rmw_opensplice_cpp/lib/librmw_opensplice_cpp.a
install_isolated//rosidl_generator_c/lib/librosidl_generator_c.a
install_isolated//rosidl_typesupport_cpp/lib/librosidl_typesupport_cpp.a
install_isolated//rosidl_typesupport_introspection_c/lib/librosidl_typesupport_introspection_c.a
install_isolated//rosidl_typesupport_introspection_cpp/lib/librosidl_typesupport_introspection_cpp.a
install_isolated//rosidl_typesupport_opensplice_c/lib/librosidl_typesupport_opensplice_c.a
install_isolated//rosidl_typesupport_opensplice_cpp/lib/librosidl_typesupport_opensplice_cpp.a
...

Just for curiosity, what's the rationale for the typesupport refactor? The description doesn't state it and can't find a discussion about it (not saying that there isn't, just that I can't find it).

Building ROS 2 without ament_tools is not discouraged. It should be users choice.

The original idea of the architecture was that the code above the rmw interface is agnostic to the used rmw implementation (see http://design.ros2.org/articles/ros_middleware_interface.html#why-should-the-middleware-interface-be-agnostic-to-dds). That is currently only true for the sources. But when the sources get built the whole stack from rcl up to the user code is being compiled N times - for each rmw implementation a separate lib / exe is created. While that was not the case in the prototypes a year ago we sacrificed this in order to move forward with supporting all platforms.

This results in a major overhead in every CMake file (e.g. see https://github.com/ros2/system_tests/blob/bc1dbab1787f7a77dcf1d25b329f5b3215e82b83/test_communication/CMakeLists.txt#L333 https://github.com/ros2/system_tests/blob/bc1dbab1787f7a77dcf1d25b329f5b3215e82b83/test_communication/CMakeLists.txt#L299-L303). And now we are moving forward on composition (building nodes as shared libraries as the recommended way to write ROS nodes and then loading them in a running process, see ros2/demos#84) and this burden becomes even more of a problem since the running process needs to select the matching library variation of a node which matches its rmw implementation.

Therefore the goal is to get back to the original design that anything above rmw actually only needs to be built once.

@dirk-thomas Ah, I see, thanks for the detailed explanation. It's unfortunate that this patch comes at a bad time, during all this refactoring. Maybe it's best to put this on hold until all the typesupport work is done and then find a way to accommodate the static linking usecase.

@dirk-thomas just one question, wouldn't the static linking usecase be akin to manual composition (from the demos you linked)? That is, the user knows what rmw they want to use, and just makes that decision at link time. If that's the case, maybe this PR could just be merged as is, since it simply defaults to shared linking (so on the surface nothing changes), but allows the user to decide otherwise. I'd argue that someone who tinkers with the CMake flags, knows that they are doing it at their own risk and not following the recommended path and that certain features may not work, but it's up to the user.

For example, I picked FastRTPS in the iOS examples as the rmw implementation (https://github.com/esteve/ros2_ios_examples/blob/master/ros2_listener_ios/Listener/Listener.xcodeproj/project.pbxproj#L10-L28) knowing that the examples would only work with one single rmw implementation because there's no other supported opensource rmw implementation that can be linked statically.

From my point of view the build tool should not set BUILD_SHARED_LIBS. Instead this should happen the same when building packages "by hand" (by invoking cmake manually).

Therefore I don't think this patch should be merged into ament_tools.

@dirk-thomas unfortunately the solution you describe doesn't seem to work (maybe I'm missing something) if the project creates libraries as shared explicitly. Given a CMake project like the following:

project(foo)

add_library(${PROJECT_NAME} SHARED src/foo.cpp)

the BUILD_SHARED_LIBS flag won't have any effect, which is the case for all the ROS2 libraries.

The scenario I have is as following:

• iOS apps must be built statically, any app that uses shared libraries will fail to find its libraries at runtime
• All the ROS2 libraries are explicitly created as shared via add_library(... SHARED ...)

so that's why I submitted this pull request, along with several others that remove the SHARED keyword from add_library, see for example ros2/rcl#93 and ros2/rmw#81

The changes in this PR allow the user to pass BUILD_SHARED_LIBS to ament via --cmake-flags, so they can control whether they want or not to build ROS2 libraries as static or as a shared. However, since CMake by default assumes static if BUILD_SHARED_LIBS is not present, I made it the default to avoid breaking anything.

I made the changes in ament_tools because I didn't see any other obvious place to make them, but maybe there's somewhere else more appropriate.

This is actually the same case as ros2/rmw#34 which @codebot submitted to disable shared linking, which is fairly common for embedded devices. This PR (and the accompanying PRs) extends that idea to the rest of the ROS2 up to rcl

If you could create a PR for the solution I described I am happy to take a look.

Obviously each add_library call must not specify SHARED / STATIC if it wants to use the option specified by BUILD_SHARED_LIBS. So removing those from (most) ROS 2 packages is fine. As mentioned above I only don't think this option should be passed by the build tool ament_tools since the behavior should be the same if the user invokes CMake directly. Therefore somewhere in ament_cmake the option should be checked and if not passed explicitly fall back to being enabled by default. Since some users might not want that default behavior it should probably be in a separate package so that users opt in to using it.

A question regarding iOS app: does it work for those apps to dlopen shared libraries at runtime (since that is what the rosidl_typesupport_cpp package does atm)?

@dirk-thomas I see. Would the changes in ros2/rcl@8271e41 suffice? If AMENT_HONOR_BUILD_SHARED_LIBS is passed as a CMake flag, the library will be built as static or shared depending on the value of BUILD_SHARED_LIBS, but if not, everything stays the same. I think this could be moved into its own function / macro (e.g. ament_add_library) into ament_cmake

A question regarding iOS app: does it work for those apps to dlopen shared libraries at runtime (since that is what the rosidl_typesupport_cpp package does atm)?

rclobjc only uses the C typesupport machinery, since it sits above rcl. In any case, dlopen only works for frameworks (in Apple-speak), so I can't use it either for loading typesupport libraries (at least not without further changes), but it's not a problem because I only use what rcl provides.

Would the changes in ros2/rcl@8271e41 suffice? If AMENT_HONOR_BUILD_SHARED_LIBS is passed as a CMake flag, the library will be built as static or shared depending on the value of BUILD_SHARED_LIBS, but if not, everything stays the same. I think this could be moved into its own function / macro (e.g. ament_add_library) into ament_cmake

While that would work I don't think it is reasonable to expect every single package to perform that conditional add_library call. We are trying hard to make the CMake part in ROS 2 as easy to use as possible (that is why we are reworking the typesupport in the first place - to not expose it to the user anywhere).

A question regarding iOS app: does it work for those apps to dlopen shared libraries at runtime (since that is what the rosidl_typesupport_cpp package does atm)?

rclobjc only uses the C typesupport machinery, since it sits above rcl. In any case, dlopen only works for frameworks (in Apple-speak), so I can't use it either for loading typesupport libraries (at least not without further changes), but it's not a problem because I only use what rcl provides.

rosidl_typesupport_c will soon do the same thing as rosidl_typesupport_cpp namely loading the specific typesupport on demand using poco / dlopen. Only after that we can invert the dependencies and work on a "shortcut" to select the typesupport for a single rmw implementation at compile time (which could link statically again and bypass the dlopen logic since only one option is available anyway). These steps are mentioned under the last bullet in the beta 1 ticket.

So as far as I understand the restrictions for iOS it won't work until we have completed the typesupport refactoring. Therefore I would suggest to wait with the shared / static library option until after that refactoring has been completed. Then we add a package (e.g. ament_cmake_ros the name is just to clarify the context and doesn't mean it has to be names like that) which set BUILD_SHARED_LIBS to TRUE by default if it is not set explcitly. Then all packages can drop their SHARED keyword and depend on this package which enable to toggle the library type.

The new package ament_cmake_ros implements the custom default value for the CMake variable BUILD_SHARED_LIBS as described in my previous comment (see ros2/ros2#304).

Therefore I think this can be closed?

@dirk-thomas thanks for adding ament_cmake_ros! Closing this PR and removing the branch.

I'll update the other branches to use ament_cmake_ros in a moment.

@esteve we may lose track of the connected PRs now that this one is closed (disappearing in the waffle Done column)

@mikaelarguedas I've created a new issue (ros2/ros2#306) to connect the corresponding PRs and updated their descriptions, hopefully it'll show up on appropriate column on the waffle board.